Skip to content

Add composite workflow action for checking user permissions prior to updating snapshots#268

Merged
brichet merged 6 commits intojupyterlab:mainfrom
martinRenou:check_perm
Jan 14, 2026
Merged

Add composite workflow action for checking user permissions prior to updating snapshots#268
brichet merged 6 commits intojupyterlab:mainfrom
martinRenou:check_perm

Conversation

@martinRenou
Copy link
Member

@martinRenou martinRenou commented Jan 13, 2026

martinRenou and others added 2 commits January 13, 2026 16:27
Co-authored-by: Nicolas Brichet <32258950+brichet@users.noreply.github.com>
@martinRenou
Copy link
Member Author

Thanks @brichet !

I've been able to confirm the reusable worfklow works as expected from that PR

@mfisher87
Copy link
Member

This is technically a "composite action", not a "reusable workflow" 😝 (sorry, I just expected something different based on the PR title when Martin and I were chatting on a call this morning!)

https://docs.github.com/en/actions/concepts/workflows-and-actions/reusing-workflow-configurations#reusable-workflows-versus-composite-actions

Copy link
Contributor

@brichet brichet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @martinRenou, LGTM.

@martinRenou martinRenou changed the title Add reusable workflow for checking user permissions prior to updating snapshots Add composite workflow action for checking user permissions prior to updating snapshots Jan 14, 2026
@martinRenou
Copy link
Member Author

Thanks a lot for the review and suggestion @brichet ! Let's hold before merging, @krassowski had some suggestion in jupyterlab/jupyterlab#18334, looking into it now.

@mfisher87 thanks! I renamed the PR

@martinRenou
Copy link
Member Author

Done and checked again that it works.

pushed_at="$(echo "$pr" | jq -r .pushed_at)"

if [[ $(date -d "$pushed_at" +%s) -gt $(date -d "$COMMENT_AT" +%s) ]]; then
echo "Updating is not allowed because the PR was pushed to (at $pushed_at) after the triggering comment was issued (at $COMMENT_AT)"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe for a follow up PR, but I wonder if we could add a comment or replace the previous 👍 with a 👎, to have a visual indication that something did not work. Same for the step below.

Otherwise it looks good, thanks.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense! Yes a comment saying the bot failed, with a link to the logs, would be great. Plus, the link to the logs could probably always show up in a comment, so that users know where to look for.

Agreed to make that as a follow-up 👍🏽

@brichet
Copy link
Contributor

brichet commented Jan 14, 2026

Thanks @martinRenou.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants